Skip to content

api: Fix initializers with mpi - #3034

Open
mloubout wants to merge 2 commits into
mainfrom
fix-mpi-padinit
Open

mloubout wants to merge 2 commits into
mainfrom
fix-mpi-padinit

Conversation

@mloubout

Copy link
Copy Markdown
Contributor

No description provided.

@mloubout mloubout added the API api (symbolics, types, ...) label Sep 24, 2026
@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 29.47368% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.84%. Comparing base (4f386a6) to head (e8d48e5).

Files with missing lines Patch % Lines
devito/builtins/initializers.py 33.33% 25 Missing and 3 partials ⚠️
tests/test_mpi.py 15.62% 27 Missing ⚠️
tests/test_data.py 16.66% 10 Missing ⚠️
devito/builtins/utils.py 50.00% 1 Missing ⚠️
devito/data/data.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3034      +/-   ##
==========================================
- Coverage   83.93%   83.84%   -0.10%     
==========================================
  Files         258      258              
  Lines       55676    55762      +86     
  Branches     4772     4785      +13     
==========================================
+ Hits        46732    46752      +20     
- Misses       8128     8191      +63     
- Partials      816      819       +3     
Flag Coverage Δ
pytest-gpu-aomp-amdgpuX 68.63% <41.17%> (-0.06%) ⬇️
pytest-gpu-gcc- 78.48% <29.47%> (-0.09%) ⬇️
pytest-gpu-icx- 78.41% <29.47%> (-0.09%) ⬇️
pytest-gpu-nvc-nvidiaX 69.17% <41.17%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mloubout mloubout changed the title Fix mpi padinit api: Fix initializers with mpi Sep 24, 2026
Comment thread devito/types/dimension.py
if tkn is None:
tkn = 0
else:
loc_size = distributor.shape[

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like distributor.shape wants to be a DimensionTuple..

Comment thread devito/types/dimension.py
# and then add 1 to get the appropriate thickness
# Dimension is of type `left`/`right` - compute the offset and
# then add 1 to get the appropriate thickness. `glb_to_loc`
# saturates to the local size when the layer covers the whole

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea what these two lines mean

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure we're not breaking the contract here?

https://github.com/devitocodes/devito/blob/main/devito/types/dimension.py#L708-L710

if we break the contract, we should rather throw an exception

Comment thread tests/test_mpi.py
assert np.all(glb[:, -3:] == 2.)

@pytest.mark.parallel(mode=[(3, 'basic'), (4, 'basic')])
def test_initialize_function_pad_over_rank(self, mode):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test belongs to test_builtins.py


def _global_plane(function, axis, index):
"""
The plane of `function` at global position `index` along `axis`, assembled

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this docstring -- can u add an example?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

"""
Replicate the boundary planes of `function` outwards into its padding.

Used in place of the symbolic extension when a rank owns no interior to read

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I don't understand exactly, but is this to cover the case in which one of the ranks at the border spans the whole left/right thickness , in practice, and so it doesn't have enough interior points to "apply padding" ? and when u say "padding", wdym exactly here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the opposite. This is whena rank is smaller than the thickness so the edge value needed to pad belongs to a different rank. The rank doesn't see the interior



def _initialize_function(function, data, nbl, mapper=None, mode='constant'):
def _rank_without_interior(function, nbl):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is distributor.loc_empty of use here or is this subtly different? Is any of the functionality attached to Distributor/SubDistributor useful here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not empty, it's smaller than the biundary layer so it doesn't have access to the edge hyperplane to pad


def _global_plane(function, axis, index):
"""
The plane of `function` at global position `index` along `axis`, assembled

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API api (symbolics, types, ...)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants